virtual_network/migrate_with_ethernet: enable host iface settings #6779
Conversation
host_iface: was handled in code but never read from config; add to config remote_host_iface: was always discovered; now make configurable default behavior maintained Signed-off-by: Sebastian Mitterle <[email protected]>
WalkthroughThis pull request modifies the Ethernet interface migration test by adding two new configuration parameters ( Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Comment |
|
running tests now |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In
`@libvirt/tests/src/virtual_network/migrate/migrate_with_ethernet_interface.py`:
- Around line 183-189: The cleanup uses unpr_user directly in shell-interpolated
commands (remote_session.cmd and process.run), which risks injection; change
both calls to avoid shell string interpolation by passing argv lists (e.g., pass
["pkill", "-u", unpr_user] and ["userdel", "-f", "-r", unpr_user] to process.run
with shell=False) or, if the remote session API only accepts a command string,
safely escape the username with shlex.quote before composing the command; update
the remote_session.cmd invocation and the process.run call to use these safe
forms and ensure ignore_status behavior is preserved.
| remote_session.cmd(f"pkill -u {unpr_user};userdel -f -r {unpr_user}") | ||
| remote_session.close() | ||
| process.run(f'pkill -u {unpr_user};userdel -f -r {unpr_user}', | ||
| shell=True, ignore_status=True) | ||
| process.run( | ||
| f"pkill -u {unpr_user};userdel -f -r {unpr_user}", | ||
| shell=True, | ||
| ignore_status=True, | ||
| ) |
There was a problem hiding this comment.
Avoid shell command construction with untrusted user input.
unpr_user is configurable; interpolating it into shell strings risks command injection. Prefer argv lists and/or quoting.
🔒 Suggested fix (avoid shell strings)
@@
-import logging
+import logging
+import shlex
@@
- if remote_session:
- remote_session.cmd(f"pkill -u {unpr_user};userdel -f -r {unpr_user}")
+ if remote_session:
+ safe_user = shlex.quote(unpr_user)
+ remote_session.cmd(f"pkill -u {safe_user}; userdel -f -r {safe_user}")
remote_session.close()
- process.run(
- f"pkill -u {unpr_user};userdel -f -r {unpr_user}",
- shell=True,
- ignore_status=True,
- )
+ process.run(["pkill", "-u", unpr_user], ignore_status=True)
+ process.run(["userdel", "-f", "-r", unpr_user], ignore_status=True)🧰 Tools
🪛 Ruff (0.14.13)
185-185: Function call with shell=True parameter identified, security issue
(S604)
🤖 Prompt for AI Agents
In `@libvirt/tests/src/virtual_network/migrate/migrate_with_ethernet_interface.py`
around lines 183 - 189, The cleanup uses unpr_user directly in
shell-interpolated commands (remote_session.cmd and process.run), which risks
injection; change both calls to avoid shell string interpolation by passing argv
lists (e.g., pass ["pkill", "-u", unpr_user] and ["userdel", "-f", "-r",
unpr_user] to process.run with shell=False) or, if the remote session API only
accepts a command string, safely escape the username with shlex.quote before
composing the command; update the remote_session.cmd invocation and the
process.run call to use these safe forms and ensure ignore_status behavior is
preserved.
1 participant
host_iface: was handled in code but never read from config; add to config
remote_host_iface: was always discovered; now make configurable
default behavior maintained
also apply 'black' formatting for CI
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.